Conversation
| } | ||
| myreader->writer()->dealloc_reader(myreader); | ||
|
|
||
| if (frame.header().flags & HTTP2_FLAGS_DATA_END_STREAM) { |
There was a problem hiding this comment.
Why are we treating read_vio.ntodo() == 0 the same as getting a data end stream flag?
There was a problem hiding this comment.
Protecting ourselves from ill-written clients that do not send DATA_END_STREAM. We have seen such clients in our production data stream. In that case they are sending Content-length, so we can still send READ_COMPLETE when all of the data is written.
|
While I understand ATS should protect itself, I don't think adding the condition is a fundamental solution. What are we going to do if we support trailing header? I assume trailing header would be written to the same VIO. Can we do READ_COMPLETE twice? I don't understand how lack of READ_COMPLETE event causes a crash, but I guess the logic that assumes READ_COMPLETE is already received is perhaps what we should fix. If this is a bandaid and we are going to take another approach when we support trailing header, it should be stated on the code or be filed as an issue. |
zwoop
added
the
OnDocs
zwoop
removed
the
OnDocs
5 participants
Breaking apart the PR in #6401.
This PR concentrates on fixing the read_vio.ndone tracking so the read_complete is sent to the HttpTunnel in a timely manner. Needs to be applied with PR #6408 to address the crashing issue.